Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace Wappalyzer #2727

Merged
merged 26 commits into from
Apr 8, 2024
Merged

Replace Wappalyzer #2727

merged 26 commits into from
Apr 8, 2024

Conversation

ammar92
Copy link
Contributor

@ammar92 ammar92 commented Mar 26, 2024

Changes

This implements python-Wappalyzer that replaces the Wappalyzer boefje and normalizer. Unfortunately, the most recent version of that package has not yet been released on pypi, hence the git dependency in the pyproject file.

Issue link

Closes #2533

Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
  • I have written unit tests for the changes or fixes I made.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have made corresponding changes to the documentation, if necessary.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

@ammar92 ammar92 marked this pull request as ready for review March 26, 2024 14:46
@ammar92 ammar92 requested a review from a team as a code owner March 26, 2024 14:46
@ammar92 ammar92 force-pushed the feature/replace-wappalyzer branch from 65b4f54 to 7fb65be Compare March 26, 2024 15:00
@ammar92
Merge branch 'main' into feature/replace-wappalyzer
f0b6a45 
# Conflicts:
#	boefjes/boefjes/plugins/kat_webpage_analysis/analyze_page/normalizer.json
#	boefjes/boefjes/plugins/kat_webpage_analysis/boefje.json
#	boefjes/boefjes/plugins/kat_website_software/boefje.json
@ammar92 ammar92 added boefjes Issues related to boefjes python labels Mar 27, 2024
@originalsouth
Copy link
Contributor

Seems to work as advertised.

@dekkers
Copy link
Contributor

dekkers commented Apr 4, 2024

Looks good to me, moved to QA

@stephanie0x00
Copy link
Contributor

Checklist for QA:

  • I have checked out this branch, and successfully ran a fresh make reset.
  • I confirmed that there are no unintended functional regressions in this branch:
    • I have managed to pass the onboarding flow
    • Objects and Findings are created properly
    • Tasks are created and completed properly
  • I confirmed that the PR's advertised feature or hotfix works as intended.

What works:

The new Wappalyzer works as expected, when scanned against mispo.es both 36 findings are shown on the main branch compared to this branch (DNS, nmap TCP, wappalyzer and webpage analysis boefjes are enabled).

What doesn't work:

For another added domain the number of findings are different 61 (this branch) vs 69 (main) findings when scanning mispo.es and an additional domain. The difference appears to be in the identification of some jQuery findings for the additional domain. My guess is that the javascript files are not detected based on the extension for the additional domain, as the javascript files have a .gjs extension.

Bug or feature?:

See above.

@ammar92
Copy link
Contributor Author

ammar92 commented Apr 8, 2024

For another added domain the number of findings are different 61 (this branch) vs 69 (main) findings when scanning mispo.es and an additional domain. The difference appears to be in the identification of some jQuery findings for the additional domain. My guess is that the javascript files are not detected based on the extension for the additional domain, as the javascript files have a .gjs extension.

Good observation and thanks for the review. The new implementation is not a fully compatible replacement for the current implementation, so (minor) differences are expected.

@ammar92 ammar92 merged commit 555c261 into main Apr 8, 2024
10 checks passed
@ammar92 ammar92 deleted the feature/replace-wappalyzer branch April 8, 2024 10:47
dekkers added a commit that referenced this pull request Apr 8, 2024
@ammar92 @underdarknl @dekkers
Replace Wappalyzer (#2727)
28efb66 
Co-authored-by: Jan Klopper <[email protected]>
Co-authored-by: Jeroen Dekkers <[email protected]>
@dekkers dekkers mentioned this pull request Apr 8, 2024
Merged
@stephanie0x00 stephanie0x00 mentioned this pull request Jun 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@originalsouth originalsouth originalsouth left review comments

@dekkers dekkers dekkers left review comments

@underdarknl underdarknl underdarknl approved these changes

Assignees

@ammar92 ammar92

Labels
boefjes Issues related to boefjes python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace Wappalyzer
5 participants
@ammar92 @originalsouth @dekkers @stephanie0x00 @underdarknl